Of Software Design, The Law of Demeter and Credit Card Companies

The Law of Demeter is a Software Engineering principle guiding how objects should talk to each other. From Wikipedia:

The Law of Demeter (LoD), or Principle of Least Knowledge, is a design guideline for developing software, particularly object-oriented programs ...[and] ...can be succinctly summarized as "Only talk to your immediate friends." The fundamental notion is that a given object should assume as little as possible about the structure or properties of anything else (including its subcomponents).

We Don't Need No Stinking Laws

Now, I'm no fan of laws, so I privately refer to this as the General Guideline of Demeter, even though it doesn't sound as snappy or cool. However, just because there are valid reasons to break it, doesn't take away from the validity of the intent. Let's look at 2 code examples blatantly ripped off from the CFCDEV mailing list:

Conforms To Law General Guideline of Demeter

view plain print about
1Room.canCustomizeWindow()
2Room.canSelectStyle()
3Room.hasCeilingFan()

Violates Law General Guideline of Demeter

view plain print about
1Room.getPlan().canCustomizeWindow()
2Room.getPlan().canSelectStyle()
3Room.getPlan().hasCeilingFan()

Ok, So What?

In the conforming set of statements, the room is asked directly whether or not certain things can happen. The implementation (steps required to complete the task) are hidden from the calling code. This is encapsulated and will help insulate callers from changes in the implementation.

In the violating set of statements, the calling code has to get a reference to Room, then ask Room for a Plan and then query the plan. Now calling code is expected to know about this Plan and what the plan knows. This adds another level of coupling and if Plan changes, then a whole lot of code has to change as well.

However, to the programmer, the violating syntax (Room.getPlan().canSelectStyle()) could make sense. It might be that the programmer doesn't want to refactor Room and using getPlan() is a faster way to do something. If the code works, is it wrong?

I Don't Follow All This Abstract Stuff. You Are Losing Me!

Ok, fair enough. My eyes glaze over with too much abstract stuff too. Let's look at an analogy.

I made a call to a credit card company. The essence of the call was:

CC Rep: CreditCardRep.answerPhone()
Thanks for calling Law Of Demeter Credit Card Company, how may I help you?

Me: Hi, My name is Dan Wilson. I have a question as to a charge on my latest bill.

CC Rep: Hi Dan, I'm Tracy. I can help you with that. For security purposes, what is your account number, mothers maiden name and shoe size?

Me: Acct: 333-444-555-5555 mothers maiden: Stratulat Shoe Size: 9

CC Rep: CreditCardRep.verifyAccountInformation()
     Perfect. What is the charge you wish to inquire about?

Me: I have a 17.99 charge to ILovePets.com on Feb 7th. I can't find my receipt so I don't know what this is for.

CC Rep: CreditCardRep.lookUpTransaction() From the transaction details, you purchased a red dog sweater. Do you recall that purchase?

Me: (Embarassed) Yeah. ok No problem. I also wanted to change my mailing address. Can you help me with that?

CC Rep: Of course, what is the new address?

Me: 123 ColdFusion Lane, Surf City, North Carolina.

CC Rep: CreditCardRep.updateAccountAddress()
     Ok. I've updated the address. Is there anything else I can help you with?

Me: No thanks. That takes care of me.. have a nice day!

CC Rep: OK Dan. Thanks for calling Law Of Demeter Credit Card Company. Have a nice day.

What Am I Supposed To Get From That Example?

Note how I called the CC company and talked with a representative. I only spoke to that representative and was not exposed to any implementation nor had to talk to any other objects to get my tasks done. Let's look at the converse example:



CC Rep: CreditCardRep.answerPhone() Thanks for calling Demeter Violation Credit Card Company, how may I help you?

Me: Hi, My name is Dan Wilson. I have a question as to a charge on my latest bill.

CC Rep: Hi Dan, I'm Tracy. I can help you with that. For security purposes, what is your account number, mothers maiden name and shoe size?

Me: Acct: 333-444-555-5555 mothers maiden: Stratulat Shoe Size: 9

CC Rep: CreditCardRep.verifyAccountInformation()
     Perfect. I can refer you to our Payment Inquiry Department. May I place you on hold?

Me: Ummm... ok.

CC Rep: Perfect. CreditCardRep.getPaymentInquiryRep() ......

CC Rep 2: Hello, this is Jessica, the Payment Inquiry ObjectRepresentative. For security purposes, what is your account number, mothers maiden name and shoe size?

Me: (Grumbles... didn't I already say this once?) Acct: 333-444-555-5555 mothers maiden: Stratulat Shoe Size: 9

CC Rep 2: CreditCardRep.getPaymentInquiryRep().verifyAccountInformation()
     Perfect. What is the charge you wish to inquire about?

Me: I have a 17.99 charge to ILovePets.com on Feb 7th. I can't find my receipt so I don't know what this is for.

CC Rep 2: CreditCardRep.getPaymentInquiryRep().lookUpTransaction()
     From the transaction details, you purchased a red dog sweater. Do you recall that purchase?

Me: (Embarassed), yeah. ok No problem. I also wanted to change my mailing address. Can you help me with that?

CC Rep 2: No, I am sorry. Our Address Change department handles that. Would you mind if I placed you on hold?

Me: (Grumbles, looks at watch) Ummm... ok.

CC Rep 2: Perfect. CreditCardRep.getPaymentInquiryRep().getAddressChangeRep() ......

CC Rep 3: Hello, this is Ann, the Address Change ObjectRepresentative. For security purposes, what is your account number, mothers maiden name and shoe size?

Me: (Grumbles... Kicks Dog) Acct: 333-444-555-5555 mothers maiden: Stratulat Shoe Size: 9

CC Rep 3: CreditCardRep.getPaymentInquiryRep().getAddressChangeRep().verifyAccountInformation() Perfect. What is the new address?

Me: 123 ColdFusion Lane, Surf City, North Carolina.

CC Rep 3: CreditCardRep.getPaymentInquiryRep().getAddressChangeRep().updateAccountAddress()
     Ok. I've updated the address. Is there anything else I can help you with?

Me: No thanks. That takes care of me.. have a nice day!

CC Rep 3: OK Dan. Thanks for calling Demeter Violation Credit Card Company. Have a nice day.

And What Shall I Gain From That Example?

I'm sure you have had a similar experience calling a credit card company. Did you feel like their processes were well designed? Wasn't it a much cleaner experience to just deal with the main object and let it handle the implementation of getting the tasks done?

Circling back to the Law of Demeter, note this passage from Wikipedia:

When applied to object-oriented programs, the Law of Demeter can be more precisely called the "Law of Demeter for Functions/Methods" (LoD-F). In this case, an object A can request a service (call a method) of an object instance B, but object A cannot "reach through" object B to access yet another object, C, to request its services. Doing so would mean that object A implicitly requires greater knowledge of object B's internal structure. Instead, B's class should be modified if necessary so that object A can simply make the request directly of object B, and then let object B propagate the request to any relevant subcomponents. Or A should have a direct reference to object C and make the call directly. If the law is followed, only object B knows its own internal structure.
So if I am Object A, should I really be exposed to the fact that the credit card company even has a Payment Inquiry Department or an Address Change department? Surely these internal details should be kept inside the CC Company Object, not sprinkled through all the various objects that interact with a Credit Card Company.

What Should I Take Away From This Nonsensical Post?

The call dialogue examples above are contrived, I admit, but think of the above dialogues as a set of design requirements for software. Now ask yourself the following questions:

  • What would have to be updated in each of the designs if the CC company added an account verification department that validated account number, mothers maiden name and shoe size?
  • What would have to be updated in each of the designs if the CC Company merged the Payment Inquiry department with the Address Change Department?
  • Which design better encapsulates the implementation from the caller?
  • Which design incurs less ripple effect from design changes?

Thoughts, concerns, comments? Add them below!

CFQueryparam and Lists

A word on SQL Injection

SQL Injection is a pervasive problem in the Web Application World. A quick search for URLS that use raw SQL brings up hundreds of thousands of dangerously formed URLS. Any developer worth his salt knows to clean user input before using it.

Defend Against SQL Injection in ColdFusion

CFQueryparam is a recommended tag that helps to keep your queries safe from SQL Injection. Any ColdFusion worth his salt uses CFQueryparam to help keep malicious parameters from being executed by the database engine. I ran across some code today that used CFQueryparam in most cases, but there was a particular, recurring use case that used raw parameters.

Example 1

view plain print about
1<cfquery name="getProductsByList" datasource="ILikeTwinkies">
2SELECT productName,
3FROM product
4WHERE productID IN ( #productIDList# )
5</cfquery>

Note the use of the list. It is a common paradigm to pass a delimited list of data to an SQL statement. In this case, the developer chose not to use CFQueryparam because he/she was under the impression that the result would be a single parameter, not a chain of parameters.

However, CFQueryparam can be used successfully in this case by setting the optional attribute 'list' to true. This is a supported attribute on all database engines.

Example 2

view plain print about
1<cfquery name="getProductsByList" datasource="ILikeTwinkies">
2SELECT productName,
3FROM product
4WHERE productID IN (<cfqueryparam value="#productIDList#" list="true" cfsqltype="cf_sql_numeric">" )
5</cfquery>

The resulting query will be parametrized in such a way as to render the list as a list and the results of the second query are equal to the first. Except for the case of an SQL Injection attack.

In the case of an SQL Injection attack, the developer of the first code sample would have a lot of explaining to do...

Must Read: Matt Woodward on Refactoring Yourself In A Corner

Matt Woodward has a keen mind for technical topics. I've enjoyed his work for a variety of years on a variety of topics such as Mach-II, The CF Weekly, Conference Presentations, CHUG and others. Today, Matt wrote a thought provoking piece about a situation that happened on his team. The piece is entitled Avoid Refactoring Yourself Into A Corner

Apart from being well written, this article explains how a change in the stakeholders precipitated a supposed change in requirements. Implementing this requirements change would have been lengthy and time-prohibitive. Through the benefit of a sharp team and open dialogue, Matt's crew was able to forgo a long and drawn out refactoring.

Refactoring is a great way to make incremental improvements to a code base, or to deal with shifting requirements. Often refactoring requires a deep understanding of many ongoing processes and the dependancies between each. All of this complication can obscure possible solutions. As Matt points out:

It's easy when you're neck deep in an application to have your thinking get a bit rigid and not be able to see the numerous other ways in which the problem you're facing might be solved. So you start going down straight-line path you see in front of you and figuring out ways to tweak what's already there to handle a new requirement that may well be the antithesis of what your domain model was originally designed to address.

I challenge you to give Avoid Refactoring Yourself Into A Corner a read and then have an honest think about similar situations you've been in before.

Practical Refactoring In ColdFusion - Live Example

Practical Refactoring

I've done a few presentations on refactoring in ColdFusion and in those presentations, I show a lot of code samples towards the end. No matter how eloquent I am in the first part of the presentation, the light always goes on for the audience when I show code. If you think about it, it makes sense, why try to explain concrete principles in abstract terms?

I do my refactoring in sweeps. A sweep is a cycle through the code in which I make incremental improvements. At the end of a sweep, the code should (it better) still function as well as it did pre-sweep. By breaking up the refactoring up into small sweeps, we can stay focused and keep out of the weeds.

In this article, we are going to look at some code that could use a refactor. Then we'll make a few sweeps through the code to tidy it up a bit. Along the way, I'll explain why I am refactoring out a certain piece. Keep in mind, we are all still learning. None of this is meant as Final Law of Software Design. I'm always refining my viewpoints and learning new things so I'll probably disagree with something I've done here at some point in the future. In the end, as long as this article is thought provoking for you, we've both won. Can Ya Dig?

First, here is the code sample. Scan the code and fix in your mind the general idea of what is going on. We'll pick back up on the other side.

The Original Code

view plain print about
1<cffunction name="onError" output="true">
2    <cfargument name="exception" required="true"/>
3    <cfargument name="eventName" required="true"/>
4        <cfif compareNoCase(trim(arguments.eventName),"onSessionEnd") And compareNoCase(trim(arguments.eventName),"onApplicationEnd")>
5            <cfif compareNoCase( arguments.Exception.RootCause.Type, "coldfusion.runtime.AbortException") >
6                <cfoutput>
7                <CFIF isDebugMode() is false>
8                <cfinclude template="/errortemplate.htm"/>
9                <CFELSE>
10                <h2>An unexpected error has occurred.</h2>
11                <p>Error event: #arguments.eventName#</p>
12                <p>Error details:</p>
13                <cfdump var="#arguments.exception#"/>
14                </CFIF>
15                <cflog file="#this.name#" type="error" text="Event name: #arguments.eventName#"/>
16                <cflog file="#this.name#" type="error" text="Message: #arguments.exception.message#"/>
17                </cfoutput>
18            </cfif>
19        <cfelseif not compareNoCase(arguments.eventName,"onApplicationEnd")>
20            <cflog file="#this.name#" type="Information" text="Application #this.name# ended."/>
21            <cflog file="#this.name#" type="error" text="Event name: #arguments.eventName#"/>
22            <cflog file="#this.name#" type="error" text="Message: #arguments.exception.message#"/>
23        </cfif>
24</cffunction>

Summary

Ok, answer to yourself the following questions:

  • Is the code right?
  • Do you know the purpose of the code?
  • Is it easy to understand?

The first question is a bit of a slippery slope. In my book 'Right' and 'Wrong' are determined by whether or not the code works. The code above actually works just fine. Compiled down to Java Bytecode, it happily processes errors with no defects, so it is 'Right'. When dealing with fuzzy subjects like code-cleanliness and proper organization, you'll find many more opinions than hard/fast rules. Clean Code is like Good Art, you know what you like when you see it. So stay away from calling other developers' code 'Wrong'. It only makes people defensive and defensive minds are not in learning mode.

Now that we've gotten past that, did you understand the purpose of the code? If you said "this code handles errors", give yourself 2 points. (The function name helped, didn't it?)

Was the code easy to understand? Can you verbalize what the code is doing? Try... no seriously, read this code from top to bottom. Can you do it? Did you have to reread parts of it?

It is a known fact that code we write personally instintively makes more sense to us than code other people write. This holds true even months after the code has been written. Good programmers write code that can be easily be understood by others. We are going to refactor this code to be more easily read.

First Analysis

A few easy things to point out... compare() and compareNoCase() are two functions that work opposite of how we read. If you use compare() to compare two identical strings, you get a return of 0, which ColdFusion treats as false, meaning we have to reverse our boolean logic in conditionals to accomodate this. Some people will tell you that compare() is faster than using an operator like IS or IS NOT. They might be right, I'm going to refactor the use of compare() and compareNoCase() out of the code anyways. If it turns out we needed the extra 'performance' of these functions, we should have implemented this part of the site in Hardware.

Next, the case is mixed inappropriately. Some operators are lower case, some are mixed case. Some of the conditionals are upper case, some are lower case. We'll normalize this and get it all looking consistant.

Also, this code has nested conditionals. Some times the only way to represent logical flow is to nest conditionals though most often it can be simplified for better readability. In this case, it looks like we truly care about a few conditions so I'll look at untangling the nest as well.

I'll put a few comments in as well, since that will help us remember what we were thinking in the middle of the refactor. Without further preamble...

First Refactor

view plain print about
1<cffunction name="onError" output="true">
2    <cfargument name="exception" required="true"/>
3    <cfargument name="eventName" required="true"/>
4        
5        <!--- Exit conditions --->
6        <cfif arguments.Exception.RootCause.Type IS "coldfusion.runtime.AbortException">
7            <cfreturn />
8        </cfif>
9        
10        <cfif trim(arguments.eventName) IS "onSessionEnd">
11            <cfreturn />
12        </cfif>
13        <!--- Log and Exit --->
14        <cfif trim(arguments.eventName) IS "onApplicationEnd">
15            <cflog file="#this.name#" type="Information" text="Application #this.name# ended."/>
16            <cflog file="#this.name#" type="error" text="Event name: #arguments.eventName#"/>
17            <cflog file="#this.name#" type="error" text="Message: #arguments.exception.message#"/>
18            <cfreturn />
19        </cfif>
20        <!--- use error template file or print to screen --->
21        <cfoutput>
22            <cfif isDebugMode() is false>
23                <cfinclude template="/errortemplate.htm"/>
24            <cfelse>
25                <h2>An unexpected error has occurred.</h2>
26                <p>Error event: #arguments.eventName#</p>
27                <p>Error details:</p>
28                <cfdump var="#arguments.exception#"/>
29            </cfif>
30            <cflog file="#this.name#" type="error" text="Event name: #arguments.eventName#"/>
31            <cflog file="#this.name#" type="error" text="Message: #arguments.exception.message#"/>
32        </cfoutput>
33</cffunction>

Second Analysis

Ok, That is a pretty good start. We got rid of compare() and compareNoCase() so the code reads a little nicer. We got rid of the nested conditionals so we don't have to keep track of several layers of boolean logic and we cleaned up the inconsistent use of case.

Another thing that I try to stay away from is using negative boolean logic. See where we check for the isDebugMode() function? I'd much rather have the TRUE case up top and have the negative case be the ELSE portion.

Also, some of the logging is duplicated. Looks like someone was Copy/Pasting :). Let's clean that up as well.

Lastly, we are going to put in good comments all the way down the line and use appropriate whitespace for readability.

Second Refactor

view plain print about
1<cffunction name="onError" output="true">
2    <cfargument name="exception" required="true"/>
3    <cfargument name="eventName" required="true"/>
4    
5    <!--- We don't want to deal with the cflocation as an error, so bail --->
6        <cfif arguments.Exception.RootCause.Type IS "coldfusion.runtime.AbortException">
7            <cfreturn />
8        </cfif>
9    <!--- Lets deal with events next. --->
10        <!--- We don't want to deal with the onSessionEnd as an error, so bail --->
11        <cfif trim(arguments.eventName) IS "onSessionEnd">
12            <cfreturn />
13        </cfif>
14        
15        <!--- From this point onward, we want to log the errors --->
16        <cflog file="#this.name#" type="error" text="Event name: #arguments.eventName#"/>
17        <cflog file="#this.name#" type="error" text="Message: #arguments.exception.message#"/>
18
19        <!--- Log as Information and Bail --->
20        <cfif trim(arguments.eventName) IS "onApplicationEnd">
21            <cflog file="#this.name#" type="Information" text="Application #this.name# ended."/>
22            <cfreturn />
23        </cfif>
24    <!--- Now, let's deal with explicit types we want to handle... --->
25        <!--- If we get here, we have something to handle --->
26        <cfif isDebugMode() is true>
27            <!--- print the error to the screen --->
28            <cfoutput>
29                <h2>An unexpected error has occurred.</h2>
30                <p>Error event: #arguments.eventName#</p>
31                <p>Error details:</p>
32                <cfdump var="#arguments.exception#"/>
33            </cfoutput>
34        <cfelse>
35            <!--- use the nice error handler --->
36            <cfinclude template="/errortemplate.htm"/>
37        </cfif>
38</cffunction>

Final Analysis

That looks even better. Now that our conditionals use positive boolean logic they are much easier to read. We also were able to remove duplicate logging calls. Not all code duplication is bad, of course, but when we can get rid of it and improve the clarity and readability, we all win.

The comments are also very descriptive and tell us the intent and reasons for the code. This way, other developers can quickly understand what we were trying to do, even if it isn't working properly.

Just for fun, I'm going to paste the finished code below without comments and whitespace. Read through the code and notice how it reads more like English language.

Final Code - No Comments/No Whitespace

view plain print about
1<cffunction name="onError" output="true">
2    <cfargument name="exception" required="true"/>
3    <cfargument name="eventName" required="true"/>
4        <cfif arguments.Exception.RootCause.Type IS "coldfusion.runtime.AbortException">
5            <cfreturn />
6        </cfif>
7        <cfif trim(arguments.eventName) IS "onSessionEnd">
8            <cfreturn />
9        </cfif>
10        <cflog file="#this.name#" type="error" text="Event name: #arguments.eventName#"/>
11        <cflog file="#this.name#" type="error" text="Message: #arguments.exception.message#"/>
12        <cfif trim(arguments.eventName) IS "onApplicationEnd">
13            <cflog file="#this.name#" type="Information" text="Application #this.name# ended."/>
14            <cfreturn />
15        </cfif>
16        <cfif isDebugMode() is true>
17            <cfoutput>
18                <h2>An unexpected error has occurred.</h2>
19                <p>Error event: #arguments.eventName#</p>
20                <p>Error details:</p>
21                <cfdump var="#arguments.exception#"/>
22            </cfoutput>
23        <cfelse>
24            <cfinclude template="/errortemplate.htm"/>
25        </cfif>
26</cffunction>

Not to shabby, yeah? The code works just like the original sample, but it is much easier to understand the flow of the function. Also, by doing our refactoring in sweeps, we simplified the code in stages, reducing the chance we'll miss something. Now go to the last sample and read it aloud. Notice how it feels natural when you read it? This code will definitely be easier to maintain in the future, won't it? There are lots of other principles to keep in mind when refactoring. We'll look at a few more in the next article.

Agree? Disagree? Tell me all about it in the comments.

P.S. Do you have code you'd like to see refactored?

If you have a code sample you would like me to refactor, send it to me using the Contact Form on this blog. Please try to keep the code chunk below 100 lines as that seems to be the limit of clarity for a blog article.

June 12 I Present Refactoring in ColdFusion to the ColdFusion Meetup

June 12th, at 6:00 PM EST, I present Refactoring in ColdFusion to the ColdFusion meetup. This presentation helps to show how to move a procedural code base from a procedural architecture to an Object Oriented Architecture. Through the presentation, we look at some procedural code and then look at an object oriented representation. By comparing and contrasting the two code samples, the audience will get a better idea how to use Object Oriented structures and how to migrate an existing code base.

I am excited to give this presentation to the ColdFusion MeetUp. I also am excited because I have some special news to share. I'll let the proverbial feline out of the proverbial containment device during the presentation.

The website for this presentation is http://coldfusion.meetup.com/17/calendar/8088894/. We begin at 6:00 EST. (http://www.timeanddate.com/worldclock/)

Frameworks, XML and duplication

Duplicating code is Bad! When designing software, take great pains to avoid duplication of code. After all, if an application has a lot of duplicated logic, or cut/pasted processes, it is very easy for parts of the system to get out of wack. A small tweak here, a short change there and next thing you know there are bugs and inconsistencies galore.

As software engineers, we are trained to spot and avoid this duplication. There are whole mantras dedicated toward this end (DRY Don't Repeat Yourself). There are whole books dedicated towards refactoring out commonalities for the goal of improving software quality. This is a very important topic and duplication is one of the easier Code Smells to uncover.

Sometimes the same warning flags will pop into mind when working with an XML driven framework. "Should I really duplicate this line of XML repeatedly in my application?" and "Am I designing my application effectively?" are thoughts the reasonable programmer has. After all, if duplicating code is such a poor idea, why is duplicating XML configuration any different?

The reason it is different is what the underlying XML represents. It represents Code! Code that has been written and tested and debugged Code that performs vital purpose in your application. We aren't duplicating this code, we are duplicating a reference to the code. A type of shorthand, if you will.

If, for example, you have functionality for checking for an authorized user, and such code is wrapped into an XML snippet like this:

view plain print about
1<broadcasts>
2 <message name="verifyUserLogin" />
3</broadcasts>

You would be perfectly fine replicating this line of XML wherever you needed User Login Verification. The line of XML is very simple, with a clean syntax and very easy to implement without error. When the rules defining User Login Verification change, simply update the single section of code referred to by the XML snippet.

Properly designed XML configurations represent complex processes in a syntactically simple manner. So feel good about duplicating your lines of XML. Happy Coding!

Application Architecture and the elusive Right Answer

I often listen carefully to Chris Scott, one of the authors of ColdSpring. He has a keen mind for Application Architecture and Object Oriented Programming. Recently, while responding to a post on the ColdSpring-Dev list, he made a point that bears repeating. Rather than rephrase, I'd like to quote him directly:

There are tons of developers out there working on small teams that really don't need to be going crazy trying to protect their code against themselves. Larger projects, larger teams may very well set different standards[...]
-Chris Scott

So often, the 'right answer' depends on all the elements in the equation. Architecture decisions are about effectively managing trade offs. The 'right' answer could be Extensibility over Simplicity, Performance over Maintainability, Clarity over Convenience.

A shop with 4 developers delivering lots of little projects has quite different needs over an enterprise with 50 developers supporting years of mission critical code. The cost of a mistake are not equal. The contributions of an individual is not equal. The measure of success is not equal. The 'right answer' is also not equal.

It is up to you, developers and architects to constantly educate yourself on technologies and standards. Be mindful of your environment. Challenge your viewpoints. Ask questions.

As for the 'right answer' I'll quote the ever wise Mr. Corfield, "It Depends".